Skip to content

ref: Update SentryCrashReportConverter for improved nullability handling #6208

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 13 commits into from
Sep 30, 2025
Merged

ref: Update SentryCrashReportConverter for improved nullability handling #6208

merged 13 commits into from
Sep 30, 2025

Conversation

philprime
Copy link
Member

@philprime philprime commented Sep 19, 2025 •
edited by cursor bot
Loading

📜 Description

  • Renamed SentryKSCrashReportConverterTests to SentryCrashReportConverterTests for consistency.
  • Enhanced nullability checks in SentryCrashReportConverter to prevent potential crashes.
  • Added new test cases to validate handling of various edge cases in crash report conversion.

💡 Motivation and Context

Closes #6207

#skip-changelog

Note

Improves null-safety in crash report conversion, makes SentryThread.threadId optional on SDK_V9, adjusts debug-image fetching with null unwraps, and adds tests for edge cases.

  • Crash Report Conversion (SentryCrashReportConverter.m):
    • Defensive null handling for timestamp, user context merge, breadcrumbs (default category, guard timestamp type), notable addresses, and Swift crash info messages.
    • Validates thread index; ignores non-NSNumber values, allows nil (recrash), and fixes isMain calculation to be null-safe.
    • Adds fallbacks for exception type strings (mach/signal/user) when missing.
    • Builds debugMeta robustly by skipping nil imageAddress values.
  • SDK API (V9):
    • SentryThread.threadId is now NSNumber? and initWithThreadId: accepts nil.
  • ANR/Client:
    • Use SENTRY_UNWRAP_NULLABLE when fetching debug images for event.threads.
  • Tests:
    • New cases covering breadcrumb default category, invalid/nil thread indexes, and notable address nulls.
  • Project:
    • Rename test file references to SentryCrashReportConverterTests.*.

Written by Cursor Bugbot for commit 7fde234. This will update automatically on new commits. Configure here.

@philprime
ref: Update SentryCrashReportConverter for improved nullability handling
90e032c 
- Renamed SentryKSCrashReportConverterTests to SentryCrashReportConverterTests for consistency.
- Enhanced nullability checks in SentryCrashReportConverter to prevent potential crashes.
- Added new test cases to validate handling of various edge cases in crash report conversion.
@philprime
Copy link
Member Author

@cursor review

cursor[bot]

This comment was marked as outdated.

Sign in to view

@codecov Codecov
Copy link

codecov bot commented Sep 19, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 97.77778% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.800%. Comparing base (15c8f13) to head (7fde234).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
Sources/Sentry/SentryCrashReportConverter.m 97.500% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #6208       +/-   ##
=============================================
+ Coverage   86.787%   86.800%   +0.013%     
=============================================
  Files          438       438               
  Lines        37328     37350       +22     
  Branches     17434     17473       +39     
=============================================
+ Hits         32396     32420       +24     
+ Misses        4886      4885        -1     
+ Partials        46        45        -1
Files with missing lines Coverage Δ
Sources/Sentry/SentryANRTrackingIntegration.m 98.816% <100.000%> (+0.007%) ⬆️
Sources/Sentry/SentryClient.m 98.943% <100.000%> (+0.001%) ⬆️
Sources/Sentry/SentryThread.mm 100.000% <ø> (ø)
Sources/Sentry/SentryCrashReportConverter.m 96.347% <97.500%> (+0.414%) ⬆️

... and 5 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 15c8f13...7fde234. Read the comment docs.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 19, 2025 •
edited
Loading

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1211.19 ms 1231.26 ms 20.07 ms
Size 23.75 KiB 995.43 KiB 971.68 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
2b7e29d 1226.55 ms 1245.44 ms 18.89 ms
a9fac2e 1212.45 ms 1219.67 ms 7.22 ms
884b224 1221.11 ms 1255.88 ms 34.77 ms
ccf1278 1226.84 ms 1248.51 ms 21.67 ms
f92cfa9 1228.45 ms 1251.33 ms 22.88 ms
3279d4e 1215.76 ms 1256.45 ms 40.69 ms
119ab1c 1226.79 ms 1254.55 ms 27.76 ms
7f4bf81 1241.73 ms 1270.66 ms 28.93 ms
0ede342 1233.47 ms 1262.29 ms 28.82 ms
f5666e7 1227.08 ms 1260.18 ms 33.10 ms

App size

Revision Plain With Sentry Diff
2b7e29d 23.75 KiB 933.03 KiB 909.29 KiB
a9fac2e 23.75 KiB 879.53 KiB 855.78 KiB
884b224 23.75 KiB 879.55 KiB 855.80 KiB
ccf1278 23.75 KiB 877.15 KiB 853.40 KiB
f92cfa9 23.75 KiB 855.38 KiB 831.62 KiB
3279d4e 23.75 KiB 938.32 KiB 914.57 KiB
119ab1c 23.75 KiB 993.70 KiB 969.95 KiB
7f4bf81 23.75 KiB 919.70 KiB 895.95 KiB
0ede342 23.75 KiB 928.15 KiB 904.40 KiB
f5666e7 23.75 KiB 963.18 KiB 939.43 KiB

Previous results on branch: philprime/fix-crash-reporter-null-handling

Startup times

Revision Plain With Sentry Diff
94f6473 1223.04 ms 1251.38 ms 28.33 ms
81f34e6 1222.77 ms 1248.67 ms 25.90 ms
88d138b 1222.39 ms 1253.94 ms 31.55 ms
97f2294 1217.53 ms 1250.04 ms 32.51 ms

App size

Revision Plain With Sentry Diff
94f6473 23.75 KiB 977.41 KiB 953.66 KiB
81f34e6 23.74 KiB 976.91 KiB 953.17 KiB
88d138b 23.74 KiB 977.01 KiB 953.27 KiB
97f2294 23.75 KiB 980.98 KiB 957.23 KiB

@philprime philprime marked this pull request as ready for review September 22, 2025 08:51
cursor[bot]

This comment was marked as outdated.

Sign in to view

seer-by-sentry[bot]
seer-by-sentry bot reviewed Sep 22, 2025
View reviewed changes
@philprime philprime marked this pull request as draft September 22, 2025 09:46
@philprime philprime self-assigned this Sep 22, 2025
@philprime
Copy link
Member Author

@cursor review
@sentry review

philprime
philprime commented Sep 23, 2025
View reviewed changes
philprime
philprime commented Sep 23, 2025
View reviewed changes
cursor[bot]

This comment was marked as outdated.

Sign in to view

@philprime philprime marked this pull request as ready for review September 23, 2025 11:56
seer-by-sentry[bot]
seer-by-sentry bot reviewed Sep 23, 2025
View reviewed changes
philprime
philprime commented Sep 23, 2025
View reviewed changes
cursor[bot]

This comment was marked as outdated.

Sign in to view

philipphofmann
philipphofmann approved these changes Sep 29, 2025
View reviewed changes
Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@philprime philprime enabled auto-merge (squash) September 30, 2025 10:11
@philprime philprime merged commit 08fc5f9 into main Sep 30, 2025
227 of 236 checks passed
@philprime philprime deleted the philprime/fix-crash-reporter-null-handling branch September 30, 2025 11:52
philprime added a commit that referenced this pull request Oct 10, 2025
@philprime
ref: Update SentryCrashReportConverter for improved nullability handl…
c5ec6ee 
…ing (#6208)

- Renamed SentryKSCrashReportConverterTests to SentryCrashReportConverterTests for consistency.
- Enhanced nullability checks in SentryCrashReportConverter to prevent potential crashes.
- Added new test cases to validate handling of various edge cases in crash report conversion.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@seer-by-sentry seer-by-sentry[bot] seer-by-sentry[bot] left review comments

@cursor cursor[bot] cursor[bot] left review comments

@philipphofmann philipphofmann philipphofmann approved these changes

@noahsmartin noahsmartin Awaiting requested review from noahsmartin noahsmartin is a code owner

@itaybre itaybre Awaiting requested review from itaybre itaybre is a code owner

Assignees

@philprime philprime

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Fix nullability in SentryCrashReportConverter.m

3 participants

@philprime @itaybre @philipphofmann